🔍 Deposit payment triggers two separate TigerBeetle transfers for the same funds

The payDeposit handler creates a TigerBeetle pending transfer directly at server/routers/propertyEscrow.ts:414-422 (escrow account), then calls executeTransferPipeline() at line 453 which creates a second TigerBeetle pending transfer at server/_core/transferPipeline.ts:202-211 (generic debit/credit accounts). Both transfers debit the same user but to different credit accounts. The wallet is only debited once in PostgreSQL, but TigerBeetle records two separate holds. This may cause the TigerBeetle balance to diverge from the PostgreSQL balance, and the reconciliation worker would flag this as drift. The same pattern exists in payInstallment.

(Refers to lines 453-465)

Was this helpful? React with 👍 or 👎 to provide feedback.

Confirmed and fixed in #34. payDeposit was creating its own escrow-ledger createPendingTransfer and then calling executeTransferPipeline, whose step 4 created a second generic hold for the same funds — a double debit hold. I added a skipLedger option to the pipeline and set it on the deposit call, so the pipeline still runs sanctions/fraud/Kafka but no longer double-books. payInstallment does not call the pipeline, so it was already single-hold.

🔴 Transfer compensation always fails to void the pending hold because it uses a random ID instead of the real one

The pending transfer ID used for voiding is fabricated from a random value (BigInt(Date.now()) * BigInt(999) + ... at server/_core/transferPipeline.ts:361) instead of being the actual ID of the pending transfer created during the pipeline, so the void always targets a non-existent transfer and fails.

Impact: Every compensation attempt falls through to the reversal fallback path, and if the original pending transfer exists, its hold is never released — funds remain locked.

const pendingId = BigInt(Date.now()) * BigInt(999) + BigInt(Math.floor(Math.random() * 999));

The compensateFailedTransfer function at server/_core/transferPipeline.ts:358-382 needs to receive the actual pending transfer ID that was created during executeTransferPipeline. Currently, the pendingId on line 361 is fabricated from Date.now() * 999 + random, which will never match any real pending transfer.

To fix this:
1. Add a transferBigId or pendingTransferId field to the compensateFailedTransfer input parameters so callers can pass the actual pending transfer ID.
2. Use that real ID as the pendingId parameter when calling tigerBeetle.voidPendingTransfer().
3. Update all callers of compensateFailedTransfer to pass the pending transfer ID that was returned/generated during the original transfer creation.

Alternatively, if the pending transfer ID is not available to callers, consider looking it up from the transferId (which is passed as input) using the userData128 field that was set during createPendingTransfer in executeTransferPipeline.

Was this helpful? React with 👍 or 👎 to provide feedback.

Confirmed and fixed in #34. The compensation generated pendingId from Date.now()*999 + random, which never matches the id of the pending transfer created by the pipeline, so voidPendingTransfer could not release the real hold. Fixed by deriving a deterministic id from the transfer UUID in both the pipeline (when creating the hold) and the compensation (when voiding it):

function pendingTransferIdFor(transferId: string): bigint {
  const hex = transferId.replace(/-/g, "").slice(0, 32).padEnd(32, "0");
  return BigInt(`0x${hex}`);
}

🔴 Escrow deposit deducts from the buyer's wallet but does not refund if the ledger write fails afterward

The buyer's wallet balance is permanently reduced (db.update(wallets) at server/routers/propertyEscrow.ts:407-411) before the TigerBeetle ledger call, so if the now-unguarded ledger write throws, the deducted funds are never restored.

Impact: A buyer loses money from their wallet without the escrow deposit being recorded, and the plan stays in draft status.

In server/routers/propertyEscrow.ts payDeposit handler (around lines 407-422), the wallet debit is committed to the database before the TigerBeetle createPendingTransfer call. If TigerBeetle fails, the wallet debit is not rolled back.

Possible fixes:
1. Wrap the wallet debit and TigerBeetle call in a database transaction, so the wallet debit can be rolled back if TigerBeetle fails.
2. Move the TigerBeetle call before the wallet debit, so if it fails, no wallet state is modified.
3. Add a try/catch around the TigerBeetle call that rolls back the wallet debit on failure (re-credit the wallet).

The same issue exists in the payInstallment handler around lines 497-512.

Was this helpful? React with 👍 or 👎 to provide feedback.

Confirmed and fixed in #34. The wallet debit auto-commits, then createPendingTransfer runs; if the ledger hold throws, the buyer is left short with no escrow record. The TB call is now wrapped so the wallet debit is refunded before the error propagates:

try {
  await tigerBeetle.createPendingTransfer({ ... });
} catch (err) {
  await db.update(wallets).set({
    balance: sql`CAST(CAST(${wallets.balance} AS DECIMAL(18,2)) + ${depositUsd} AS VARCHAR)`,
    updatedAt: new Date(),
  }).where(eq(wallets.id, wallet.id));
  throw err;
}

🔴 Escrow installment deducts from the buyer's wallet but does not refund if the ledger write fails afterward

The buyer's wallet balance is permanently reduced (db.update(wallets) at server/routers/propertyEscrow.ts:497-501) before the TigerBeetle ledger call, so if the now-unguarded ledger write throws, the deducted funds are never restored.

Impact: A buyer loses money from their wallet without the installment being recorded, and the installment stays in "scheduled" status.

Was this helpful? React with 👍 or 👎 to provide feedback.

Confirmed and fixed in #34, same as the deposit path — the installment createPendingTransfer is now wrapped so the wallet debit is refunded if the ledger hold fails, before rethrowing.

🔴 Posting a pending transfer proceeds with empty data when the database query fails with a non-connection error

The pending transfer lookup (s.db.QueryRow at services/ledger-service/main.go:529-532) only checks for sql.ErrNoRows but not other database errors, so any other failure causes the handler to proceed with zero-value strings and create a bogus posted transfer.

Impact: A database error silently creates a transfer record with empty account IDs and zero amount, corrupting the ledger.

err := s.db.QueryRow(...).Scan(&debitID, &creditID, &amountStr, &code)
if err == sql.ErrNoRows {
    // return 404
}
// No check for other errors — falls through with zero-value strings

Was this helpful? React with 👍 or 👎 to provide feedback.

Confirmed and fixed in #34. The lookup only handled err == sql.ErrNoRows; any other DB error (e.g. ErrConnDone) fell through with empty debitID/creditID/amountStr and the handler proceeded to post a transfer against empty accounts. Added an explicit fail-closed branch:

} else if err != nil {
    log.Printf("[ledger-service] FAIL-CLOSED: post-pending lookup failed: %v", err)
    w.WriteHeader(http.StatusInternalServerError)
    json.NewEncoder(w).Encode(map[string]string{"error": "LOOKUP_FAILED", ...})
    return
}

🔍 Go and Rust ledger services perform transfer insert and balance updates as separate non-transactional statements

In handleCreateTransfer at services/ledger-service/main.go:456-483, the transfer is inserted into ledger_transfers, then four separate UPDATE statements modify account balances. These are not wrapped in a database transaction. If the process crashes or the connection drops between the transfer insert and the balance updates, the ledger will have a recorded transfer but incorrect account balances. The same pattern exists in handlePostPending (lines 560-573) and handleVoidPending (lines 623-636). The Rust service has the same issue at services/rust-tigerbeetle-service/src/main.rs:536-551. For a financial ledger service, this non-atomicity could cause balance drift that the reconciliation worker would need to detect and repair.

Was this helpful? React with 👍 or 👎 to provide feedback.

Confirmed and fixed in #34. In the Go handleCreateTransfer, the transfer INSERT and the four balance UPDATEs ran as separate statements, so a crash between them left the ledger inconsistent. They now run inside a single sql.Tx (Begin → Exec insert → balance Execs → Commit, with defer tx.Rollback()), and balance-update errors now fail closed instead of being ignored.

Same fix on the Rust side: new db_record_transfer_atomic wraps the insert + both balance updates in one sqlx transaction — and the previous Rust code only logged balance-update failures (transfer persisted while balances didn't), which now fails closed too.

🔍 Python reconciliation worker converts API float responses back to integers with potential precision loss

In services/python-tigerbeetle-reconciliation/main.py:168-171, the reconciliation worker converts float values from the TigerBeetle/Go service API responses back to integers: int(float(data.get('debits_pending', 0)) * 1_000_000). Since the API returns values divided by 1,000,000 (e.g., minorToAmount in Go), this round-trip through floating point can introduce rounding errors. For example, if the actual minor value is 999999999999999, dividing by 1M gives 999999999.999999 which when multiplied back gives 999999999999999.0 — but for certain values, the float representation may lose precision. This could cause the reconciliation worker to flag false-positive drift discrepancies.

Was this helpful? React with 👍 or 👎 to provide feedback.

Confirmed and fixed in #34. The ledger services return balances as floats, and int(float(x) * 1_000_000) truncates — e.g. 0.07 * 1_000_000 == 69999.99999999999 → 69999, off by one minor unit. For a drift-detection worker that manufactures phantom 1-unit discrepancies (or masks real ones). Replaced the truncating casts with a rounding helper applied to all four fields from both the TB and Go clients:

def _to_minor(value) -> int:
    return int(round(float(value or 0) * 1_000_000))

🟥 Negative transfer amounts bypass balance checks and reverse fund flow

The new Go ledger service CreateTransferReq struct at services/ledger-service/main.go:113 accepts Amount float64 with no minimum validation. The old code had binding:"required,min=1" which enforced positive amounts. A negative amount passes the balance pre-check at line 427 (available < amountMinor is false when amountMinor is negative), creates a transfer, and then the balance updates at lines 474-482 add a negative value to debits_posted/credits_posted, effectively reversing the money flow — the debit account gains funds and the credit account loses them.

Was this helpful? React with 👍 or 👎 to provide feedback.

Confirmed and fixed in #34. A negative Amount passed the balance pre-check (available < amountMinor is false when amountMinor is negative) and reversed the fund flow. handleCreateTransfer now rejects non-positive amounts up front:

if req.Amount <= 0 {
    w.WriteHeader(http.StatusBadRequest)
    json.NewEncoder(w).Encode(map[string]interface{}{"error": "INVALID_AMOUNT", ...})
    return
}

🟨 Hardcoded database credentials in default connection string

The Go ledger service at services/ledger-service/main.go:156 contains a hardcoded fallback database URL with credentials: postgresql://remitflow:remitflow123@localhost:5432/remitflow?sslmode=disable. The same pattern appears in the Rust service at services/rust-tigerbeetle-service/src/main.rs:235. These credentials are committed to source control.

Was this helpful? React with 👍 or 👎 to provide feedback.

Addressed in #34 as part of the negative-amount fix — the validation is enforced in the handleCreateTransfer handler (req.Amount <= 0 → 400 INVALID_AMOUNT) rather than via a struct binding tag, since this service decodes with encoding/json directly (no gin binding validator in the request path). This guarantees non-positive amounts are rejected before any balance check or persistence.

Correction to my previous reply — this thread is about the hardcoded default credentials, which is also fixed in #34. initDB no longer falls back to the remitflow:remitflow123@localhost default connection string when running in production; it now fails closed:

dbURL := os.Getenv("DATABASE_URL")
if dbURL == "" {
    if os.Getenv("NODE_ENV") == "production" || os.Getenv("GO_ENV") == "production" {
        log.Fatalf("[ledger-service] DATABASE_URL is required in production")
    }
    dbURL = "postgresql://remitflow:remitflow123@localhost:5432/remitflow?sslmode=disable" // dev only
}

The hardcoded default now only applies in local/dev, where it's a throwaway credential.

🟥 Negative transfer amounts bypass balance checks in Rust ledger service

The Rust service's TransferRequest at services/rust-tigerbeetle-service/src/main.rs:138 accepts amount: f64 without validation. The amount_to_minor function at line 207 casts to u128 via (amount * 1_000_000.0) as u128, and casting a negative float to unsigned in Rust produces 0 (saturating cast in recent Rust). This means a negative amount becomes a 0-amount transfer that passes all checks and creates a zero-value transfer record, polluting the ledger with no-op entries. More critically, the balance check at line 514 compares available < amount_minor as i128 — if amount_minor is 0, this always passes.

Was this helpful? React with 👍 or 👎 to provide feedback.

Confirmed and fixed in #34. A negative f64 cast to u128 in amount_to_minor wraps to an enormous value, and a negative amount would also bypass the balance pre-check. initiate_transfer now validates the amount before any conversion:

if !req.amount.is_finite() || req.amount <= 0.0 {
    return (StatusCode::BAD_REQUEST, Json(serde_json::json!({
        "error": "INVALID_AMOUNT", "amount": req.amount,
        "message": "Transfer amount must be greater than zero"
    })));
}